Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved ML button styling #73

Merged
merged 52 commits into from
Mar 19, 2024
Merged

Improved ML button styling #73

merged 52 commits into from
Mar 19, 2024

Conversation

AIC-BV
Copy link
Contributor

@AIC-BV AIC-BV commented Oct 3, 2023

Not perfect, but much better than it actually was. Feel free to edit however you see fit!
The button is more obvious. Users will see more easily that they have to translate the corresponding field.

AIC-BV added 2 commits October 3, 2023 13:26
Not perfect, but much better than it actually was.
The button is more obvious. Users will see more easily that they have to translate the field.
@mjauvin mjauvin self-assigned this Oct 3, 2023
@mjauvin mjauvin added this to the v2.1.5 milestone Oct 3, 2023
@mjauvin
Copy link
Member

mjauvin commented Oct 3, 2023

@AIC-BV there is a problem with markdown editor:
image

It is hiding the "preview" (eye) icon:
image

@AIC-BV
Copy link
Contributor Author

AIC-BV commented Oct 3, 2023

@mjauvin Must've overlooked it when converting the CSS to LESS (because I tested & fixed it after your comment in discord 😅)

@mjauvin
Copy link
Member

mjauvin commented Oct 3, 2023

In the .ml-btn class, can you remove height:... and add bottom: 1px; and box-shadow: none; ?

@AIC-BV
Copy link
Contributor Author

AIC-BV commented Oct 3, 2023

Didn't know adding the bottom would actually set its height correctly!
Better now @mjauvin?

@mjauvin
Copy link
Member

mjauvin commented Oct 3, 2023

Really nice, one more thing:

diff --git a/assets/less/multilingual.less b/assets/less/multilingual.less
index db20a4a..a174cc8 100644
--- a/assets/less/multilingual.less
+++ b/assets/less/multilingual.less
@@ -65,12 +65,13 @@
            
     &.field-multilingual-markdowneditor {
         .ml-btn {
-            top: 41px;
+            right: 76px;
+            background-color: transparent;
         }
 
         .ml-dropdown-menu {
-            top: 74px;
-            right: 1px;
+            top: 36px;
+            right: 76px;
         }
     }

image

@mjauvin
Copy link
Member

mjauvin commented Oct 3, 2023

Would be nice to move the ml-btn on the RichEditor as well, but it seems more difficult.

@AIC-BV
Copy link
Contributor Author

AIC-BV commented Oct 4, 2023

Really nice, one more thing:

I added this!

@mjauvin
Copy link
Member

mjauvin commented Oct 4, 2023

@AIC-BV can you paste a screenshot of what the repeater ML-button looks like here?

@AIC-BV
Copy link
Contributor Author

AIC-BV commented Oct 4, 2023

It was like this
image

but adding bottom: 1px made it like this
image

Can make it back to the first screenshot by adding
bottom: auto;
(& top: 3px;)

What do you think?

@mjauvin
Copy link
Member

mjauvin commented Oct 4, 2023

Yes, do what's required!

@AIC-BV
Copy link
Contributor Author

AIC-BV commented Oct 4, 2023

@mjauvin should be done

@mjauvin
Copy link
Member

mjauvin commented Oct 5, 2023

Dam, doesn't work on mobile
Screenshot_20231004-211533

@mjauvin
Copy link
Member

mjauvin commented Oct 5, 2023

Returns markdown editor ml-button to previous location and add margin to text input box so that we don't end-up under the ml-button

diff --git a/assets/less/multilingual.less b/assets/less/multilingual.less
index 237ba1a..aaa1dfa 100644
--- a/assets/less/multilingual.less
+++ b/assets/less/multilingual.less
@@ -69,14 +69,17 @@
     }
 
     &.field-multilingual-markdowneditor {
+        .editor-write {
+            margin-right: 2rem;
+        }
         .ml-btn {
-            right: 76px;
-            background-color: transparent;
+            top: 41px;
+            border-radius: 0;
+            border-bottom-left-radius: .375rem;
         }
 
         .ml-dropdown-menu {
-            top: 36px;
-            right: 76px;
+            top: 80px;
         }
     }

@AIC-BV
Copy link
Contributor Author

AIC-BV commented Oct 19, 2023

Needs this wintercms/winter#997 for markdowneditor

@AIC-BV
Copy link
Contributor Author

AIC-BV commented Oct 27, 2023

@mjauvin 😄

@mjauvin
Copy link
Member

mjauvin commented Oct 27, 2023

Looks like it's working fine, but it's not triggering all the times when you get back from fullscreen mode (especially if you don't paste a huge amount of text).

@AIC-BV
Copy link
Contributor Author

AIC-BV commented Nov 6, 2023

Looks like it's working fine, but it's not triggering all the times when you get back from fullscreen mode (especially if you don't paste a huge amount of text).

Can you show a video
For me it works every time but it snaps back with a delay

@mjauvin
Copy link
Member

mjauvin commented Mar 18, 2024

@AIC-BV where were we on this?

@AIC-BV
Copy link
Contributor Author

AIC-BV commented Mar 18, 2024

For me it is finished 😄
You had an issue you mentioned above but I can't reproduce it

@mjauvin
Copy link
Member

mjauvin commented Mar 18, 2024

@AIC-BV here's a video for the markdown editor that shows it is not updating:
Screencast from 2024-03-18 12-48-53.webm

@mjauvin
Copy link
Member

mjauvin commented Mar 18, 2024

I am using the Winter.Test plugin and added the following to the Post model:

    public $implement = ['@Winter.Translate.Behaviors.TranslatableModel'];
    public $translatable = ['name', 'content_md', 'content_html'];

@mjauvin mjauvin requested review from mjauvin and removed request for mjauvin March 18, 2024 18:04
@mjauvin mjauvin modified the milestones: v2.1.5, v2.1.6 Mar 18, 2024
@AIC-BV
Copy link
Contributor Author

AIC-BV commented Mar 19, 2024

@mjauvin Markdowneditor has been fixed

@mjauvin mjauvin marked this pull request as ready for review March 19, 2024 10:44
@mjauvin mjauvin merged commit 56935bb into wintercms:main Mar 19, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants